Add grouping of modules #780

LostKobrakai · 2017-09-27T09:20:20Z

Regarding #779.

By now I've just added grouping for modules. I can add the changes for the grouping of pages at a later time. Also I'm not sure where/how to test the changes to ExDoc.Formatter.HTML.Templates.

Also:
Damn is it nice to use it even just for the following groups in phoenix projects.

[
          "Web - Controllers": ~R/^MyAppWeb\..*?Controller$/,
          "Web - Views": ~R/^MyAppWeb\..*?View$/,
          "Web - Plugs": ~R/^MyAppWeb\..*?Plug$/,
          "Web - Phoenix": ~R/^MyAppWeb.?/
]

sourcelevel-bot · 2017-09-27T09:20:51Z

test/fixtures/grouped_implicitly_regex.ex

@@ -0,0 +1,2 @@
+defmodule GroupedImplicitlyRegex do


Modules should have a @moduledoc tag.

sourcelevel-bot · 2017-09-27T09:20:51Z

test/fixtures/grouped_explicitly_string.ex

@@ -0,0 +1,2 @@
+defmodule GroupedExplicitlyString do


Modules should have a @moduledoc tag.

sourcelevel-bot · 2017-09-27T09:20:51Z

test/fixtures/grouped_explicitly_atom.ex

@@ -0,0 +1,2 @@
+defmodule GroupedExplicitlyAtom do


Modules should have a @moduledoc tag.

sourcelevel-bot · 2017-09-27T09:20:52Z

test/ex_doc/retriever_test.exs

    files  = Enum.map names, fn(n) -> "test/tmp/Elixir.#{n}.beam" end
-    config = %ExDoc.Config{source_url_pattern: url_pattern, source_root: File.cwd!}
+    config = %ExDoc.Config{
+      source_url_pattern: url_pattern, 


There should be no trailing white-space at the end of a line.

sourcelevel-bot · 2017-09-27T09:20:52Z

test/ex_doc/retriever_test.exs

    [module_node] = docs_from_files ["CompiledWithDocs"]
    assert module_node.module == CompiledWithDocs
  end
+


There should be no trailing white-space at the end of a line.

sourcelevel-bot · 2017-09-27T09:20:52Z

lib/ex_doc/retriever.ex


+  defp module_group(module, group_patterns) do
+    group_patterns
+    |> Enum.find_value(fn {group, patterns} -> 


Use a function call when a pipeline is only one function long

sourcelevel-bot · 2017-09-27T09:20:52Z

lib/ex_doc/retriever.ex


-  defstruct id: nil, title: nil, module: nil, doc: nil, doc_line: nil,
-    docs: [], typespecs: [], source_path: nil, source_url: nil, type: nil
+  defstruct id: nil, title: nil, module: nil, group: nil, doc: nil, 


There should be no trailing white-space at the end of a line.

sourcelevel-bot · 2017-09-27T09:20:52Z

lib/ex_doc/retriever.ex

-  defstruct id: nil, title: nil, module: nil, doc: nil, doc_line: nil,
-    docs: [], typespecs: [], source_path: nil, source_url: nil, type: nil
+  defstruct id: nil, title: nil, module: nil, group: nil, doc: nil, 
+    doc_line: nil, docs: [], typespecs: [], source_path: nil, 


There should be no trailing white-space at the end of a line.

sourcelevel-bot · 2017-09-27T09:20:52Z

lib/ex_doc/retriever.ex


+  defp module_group(module, group_patterns) do
+    group_patterns
+    |> Enum.find_value(fn {group, patterns} -> 


There should be no trailing white-space at the end of a line.

sourcelevel-bot · 2017-09-27T09:20:52Z

lib/ex_doc/retriever.ex

+  defp module_in_patterns(module, patterns) do
+    "Elixir." <> module_string = Atom.to_string module
+
+    Enum.any?(patterns, fn pattern -> 


There should be no trailing white-space at the end of a line.

sourcelevel-bot · 2017-09-27T09:24:49Z

test/ex_doc/retriever_test.exs

    assert functions == ["plus_one/1", "plus_two/1"]
  end
-end
+end


There should be a final \n at the end of each file.

josevalim · 2017-09-27T15:52:41Z

lib/ex_doc/retriever.ex

    |> Enum.map(&get_module(&1, config))
    |> Enum.filter(&(&1))
-    |> Enum.sort(&(&1.id <= &2.id))
+    |> Enum.sort_by(&({&1.group, &1.id}))


I think we should order them by the group index in module_groups. So the first group will be first, etc. Thoughts?

By now order was alphabetically by module name, so I feel like having groups sorted alphabetically isn't to bad. Also if you look at the example above in the PR description I've used a less strict regex as last group to catch all phoenix related modules, which weren't caught by the more precise earlier ones. So the order of groups might be useful for "catch-all" cases more so than for sorting order of the groups.

But looking at the way groups worked with pages before, they are ordered by the order of first occurrence of a group, which might be a needed feature. Especially if we make the setup of both the same and deprecate the old way of grouping pages we should look out for that.

But looking at the way groups worked with pages before, they are ordered by the order of first occurrence of a group, which might be a needed feature.

Yup, precisely.

Plus if we give them control, they can keep it alphabetically. :)

I'd really like to retain the user-defined "order of matching" as well, because otherwise the regex patterns might have to be more complex in places. Anything like "group all the remaining modules starting with MyAppWeb" won't be easy if the order in the list does determine the frontend order instead of the matching order. Having a regex exclude the matches of possibly multiple regexes can be quite a pain.

But maybe that's a topic for a later enhancement to the implementation.

josevalim · 2017-09-27T15:53:31Z

lib/ex_doc/retriever.ex

+  end
+
+  defp module_in_patterns(module, patterns) do
+    "Elixir." <> module_string = Atom.to_string module


Do not assume the module name is "Elixir." because of Erlang modules. You should probably use the module id or title.

josevalim · 2017-09-27T15:54:07Z

lib/ex_doc/retriever.ex

+      case pattern do
+        %Regex{} = regex -> Regex.match?(regex, module_string)
+        string when is_binary(string) -> module_string === string
+        atom -> atom === module


Please use == and above. :)

I can certainly do this. I'm just curious, how would/should one decide between == and ===?

josevalim · 2017-09-27T15:55:49Z

test/fixtures/grouped_explicitly_atom.ex

@@ -0,0 +1,3 @@
+defmodule GroupedExplicitlyAtom do


I don't think we need to define new features. You can use the existing features and instead change the configuration every time you generate the docs. This can probably be done by changing this function to receive a keyword list that is merged into ExDoc.Config. then you just pass the module groups.

josevalim · 2017-09-27T15:56:29Z

@LostKobrakai this looks really nice, I have added some comments. Regarding the templates tests, a unit test will suffice for now, just set the group manually.

sourcelevel-bot · 2017-09-27T18:44:05Z

test/ex_doc/retriever_test.exs

-    config = %ExDoc.Config{source_url_pattern: url_pattern, source_root: File.cwd!}
+    config = 
+      %ExDoc.Config{source_url_pattern: "http://example.com/%{path}#L%{line}", source_root: File.cwd!}
+      |> struct(config)


Use a function call when a pipeline is only one function long

sourcelevel-bot · 2017-09-27T18:44:05Z

test/ex_doc/retriever_test.exs

+  defp docs_from_files(names, config \\ []) do
    files  = Enum.map names, fn(n) -> "test/tmp/Elixir.#{n}.beam" end
-    config = %ExDoc.Config{source_url_pattern: url_pattern, source_root: File.cwd!}
+    config = 


There should be no trailing white-space at the end of a line.

sourcelevel-bot · 2017-09-27T18:44:05Z

test/ex_doc/formatter/html/templates_test.exs

      source_url: source_url(),
      output: "test/tmp/html_templates"
    }
+    |> struct(config)


Use a function call when a pipeline is only one function long

sourcelevel-bot · 2017-09-27T18:44:06Z

test/ex_doc/formatter/html/templates_test.exs

    assert content =~ ~r("id":"CompiledWithDocs".*"functions":.*"example_without_docs/0")ms
    assert content =~ ~r("id":"CompiledWithDocs.Nested")ms
  end
+


There should be no trailing white-space at the end of a line.

sourcelevel-bot · 2017-09-28T19:08:56Z

test/ex_doc/group_matcher_test.exs

+      assert "Group" == match_module(patterns, :lists, ":lists")
+      assert nil == match_module(patterns, MyApp.SomeOtherModule, "MyApp.SomeOtherModule")
+    end
+


There should be no trailing white-space at the end of a line.

sourcelevel-bot · 2017-09-28T19:08:56Z

test/ex_doc/group_matcher_test.exs

+      assert nil == match_extra(patterns, "docs/introduction.md")
+    end
+  end
+end


There should be a final \n at the end of each file.

sourcelevel-bot · 2017-09-28T19:08:56Z

test/ex_doc/formatter/html_test.exs

+    content = read_wildcard!("#{output_dir()}/dist/sidebar_items-*.js")
+    assert content =~ ~r{"id":"readme","title":"README","group":"Intro"}
+  end
+


There should be no trailing white-space at the end of a line.

sourcelevel-bot · 2017-09-28T19:08:56Z

lib/ex_doc/group_matcher.ex

+      end
+    end)
+  end
+


There should be no trailing white-space at the end of a line.

sourcelevel-bot · 2017-09-28T19:08:56Z

lib/ex_doc/group_matcher.ex

+  defp match_patterns(patterns, matcher) do
+    Enum.any?(patterns, matcher) || nil
+  end
+end


There should be a final \n at the end of each file.

sourcelevel-bot · 2017-09-28T19:08:56Z

lib/ex_doc/formatter/html.ex

  end

-  defp build_extra(input, id, title, group, project_nodes, extension) do
+  defp build_extra(input, id, title, group, project_nodes, extension, config) do


Function takes too many parameters (arity is 7, max is 5).

josevalim · 2017-09-30T20:28:33Z

❤️ 💚 💙 💛 💜

sourcelevel-bot bot reviewed Sep 27, 2017

View reviewed changes

LostKobrakai force-pushed the feature/module_grouping branch from 40dcf34 to 9fbdde0 Compare September 27, 2017 09:24

sourcelevel-bot bot reviewed Sep 27, 2017

View reviewed changes

LostKobrakai force-pushed the feature/module_grouping branch from 9fbdde0 to 5fe54cd Compare September 27, 2017 09:25

Add grouping of modules

a59e346

LostKobrakai force-pushed the feature/module_grouping branch from 5fe54cd to a59e346 Compare September 27, 2017 09:27

josevalim reviewed Sep 27, 2017

View reviewed changes

LostKobrakai added 2 commits September 27, 2017 20:42

Update retriever / tests according to José's comments

f4d04fc

Add templates unit tests for groups of modules

1526947

sourcelevel-bot bot reviewed Sep 27, 2017

View reviewed changes

LostKobrakai added 3 commits September 27, 2017 20:49

Fix code formatting issues

6f0c182

Sort groups by the order of setup

50a822a

Implement the new config setup for grouping extra pages

51ca972

sourcelevel-bot bot reviewed Sep 28, 2017

View reviewed changes

Extract the matching logic into a new module

41d299a

LostKobrakai force-pushed the feature/module_grouping branch from 7ada3c5 to 41d299a Compare September 28, 2017 19:11

Rename config keys

fb72336

josevalim merged commit 9d571f0 into elixir-lang:master Sep 30, 2017

Add grouping of modules #780

Add grouping of modules #780

Uh oh!

Conversation

LostKobrakai commented Sep 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sourcelevel-bot bot Sep 27, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Sep 27, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Sep 27, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Sep 27, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Sep 27, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Sep 27, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Sep 27, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Sep 27, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Sep 27, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Sep 27, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Sep 27, 2017

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josevalim commented Sep 27, 2017

Uh oh!

sourcelevel-bot bot Sep 27, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Sep 27, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Sep 27, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Sep 27, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Sep 28, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Sep 28, 2017

Choose a reason for hiding this comment

Uh oh!

sourcelevel-bot bot Sep 28, 2017

Choose a reason for hiding this comment

LostKobrakai commented Sep 27, 2017 •

edited

Loading